wallet.signTransaction fail with error #5184
Conversation
nazarhussain
requested review from
avkos,
jdevcs,
luu-alex,
nikoulai and
spacesailor24
nazarhussain
force-pushed
the
nh/5170-sign-transaction
branch
from
c450824 to
2d1f95f
Compare
packages/web3/src/web3.ts
Outdated
| const signTransactionWithState = async (transaction: Transaction, privateKey: Bytes) => { | ||
| const tx = await prepareTransactionForSigning(transaction, this); | ||
|
|
||
| const privateKeyBytes = format({ eth: 'bytes' }, privateKey, ETH_DATA_FORMAT); | ||
|
|
||
| return signTransaction(tx, privateKeyBytes); | ||
| }; | ||
|
|
||
| const privateKeyToAccountWithState = (privateKey: Buffer | string) => { | ||
| const account = privateKeyToAccount(privateKey); | ||
|
|
||
| return { | ||
| ...account, | ||
| signTransaction: async (transaction: Transaction) => | ||
| signTransactionWithState(transaction, account.privateKey), | ||
| }; | ||
| }; | ||
|
|
||
| const decryptWithState = async ( | ||
| keystore: string, | ||
| password: string, | ||
| options?: Record<string, unknown>, | ||
| ) => { | ||
| const account = await decrypt( | ||
| keystore, | ||
| password, | ||
| (options?.nonStrict as boolean) ?? true, | ||
| ); | ||
|
|
||
| return { | ||
| ...account, | ||
| signTransaction: async (transaction: Transaction) => | ||
| signTransactionWithState(transaction, account.privateKey), | ||
| }; | ||
| }; | ||
|
|
||
| const createWithState = () => { | ||
| const account = create(); | ||
|
|
||
| return { | ||
| ...account, | ||
| signTransaction: async (transaction: Transaction) => | ||
| signTransactionWithState(transaction, account.privateKey), | ||
| }; | ||
| }; | ||
|
|
There was a problem hiding this comment.
These functions are in main web3 package for avoiding eth package dep in accounts pkg, or is there any thing else?
But I think these should be moved to accounts package as: If some one want to import signing functionality directly from accounts package the behavior will be diff.
Or if you want to keep in current way, move these functions to new file in main web3 package and document that defaults will not be filled if these functions are directly used from accounts package instead of main web3 package.
There was a problem hiding this comment.
As mentioned earlier in a discussion:
There are two approaches to do it.
- Make the signTransaction stateful accepting the context and handle rest
- Process state related stuff in the higher level object web3 or web3.eth and keep the signTransaction stateless.
I used second approach to have minimum changes in this PR.
There was a problem hiding this comment.
ok, sounds good. Could you do changes as discussed in chat.
-
changelog update (under breaking changes) for these functions exported via web3 main package will have diff behavior as compared to exported in accounts package. ( filling defaults )
-
moving these state functions in diff file in same web3 package and continue using second approach
Co-authored-by: Junaid <[email protected]>
Description
Fixes #5170
Type of change
Checklist:
npm run dtslintwith success and extended the tests and types if necessary.npm run test:covand my test cases cover all the lines and branches of the added code.npm run buildwith success.dist/web3.min.jsin a browser.CHANGELOG.mdfile in the root folder.